Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed obsolete-attribute from ConfigureNLog-methods for ILoggingBuilder #482

Merged
merged 1 commit into from
Sep 25, 2019

Conversation

snakefoot
Copy link
Contributor

@snakefoot snakefoot commented Sep 24, 2019

Reverting #257. Since it will no longer generate double logging after #459

Also fixed ILoggingBuilder.ConfigureNLog()-methods so they can work alone without IHostBuilder.UseNLog().

See also: dotnet/AspNetCore.Docs#14554

@304NotModified
Copy link
Member

I'm doubting about this. Afaik it's still bad practice to use both? Nothing goes wrong but it's still obsolete IMO

@snakefoot
Copy link
Contributor Author

The ILoggerBuilder interface is not obsolete. And you are not required to use IHostBuilder.

@snakefoot
Copy link
Contributor Author

snakefoot commented Sep 24, 2019

But it would be nice to have a better fluent-interface-integration with ILoggerBuilder and IHostBuilder to load the NLog-config-file depending on environment (Ex. NLog.Dev.config and NLog.Production.config)

Not everyone setup logging upfront before building the IHostBuilder (Ex want to load environment specific nlog.config). With NetCore3 you will have a lot of confused people that can no longer use ILoggerFactory. Instead a good alternative would be ILoggerBuilder.

@snakefoot
Copy link
Contributor Author

snakefoot commented Sep 25, 2019

I believe it should be possible to setup NLog as LoggingProvider directly on LoggerFactory.Create:

var loggerFactory = LoggerFactory.Create(builder =>
{
    builder.ConfigureNLog("NLog.config");
});
ILogger logger = loggerFactory.CreateLogger<Program>();
logger.LogInformation("Example log message");

And Microsoft recommends that logging is setup for IHostBuilder using IHostBuilder.ConfigureLogging:

public static IHostBuilder CreateHostBuilder(string[] args) =>
    Host.CreateDefaultBuilder(args)
        .ConfigureLogging(logging =>
        {
            logging.ClearProviders();
            logging.ConfigureNLog("NLog.config");
        })
        .ConfigureWebHostDefaults(webBuilder =>
        {
            webBuilder.UseStartup<Startup>();
        });

@304NotModified
Copy link
Member

304NotModified commented Sep 25, 2019

Ok merged! thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ASP.NET Core ASP.NET Core - all versions enhancement size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants